add flow for checking previously used accounts #40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mirzemehdi
reviewed
Jul 29, 2024
kmpauth-google/src/androidMain/kotlin/com/mmk/kmpauth/google/GoogleAuthUiProviderImpl.kt
Outdated
Show resolved
Hide resolved
|
@bom-bakbang thank you for providing detailed info and your contribution, I requested some small changes. Please, check them |
mirzemehdi
requested changes
Jul 29, 2024
kmpauth-google/src/androidMain/kotlin/com/mmk/kmpauth/google/GoogleAuthUiProviderImpl.kt
Outdated
Show resolved
Hide resolved
mirzemehdi
reviewed
Jul 29, 2024
kmpauth-google/src/androidMain/kotlin/com/mmk/kmpauth/google/GoogleAuthUiProviderImpl.kt
Outdated
Show resolved
Hide resolved
- move repeated logics to one function - change parameter name
Thanks for detail code review. Sorry for late correction changes, I got accident so I couldn't focus 😅. |
mirzemehdi
reviewed
Oct 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi! I'm Lukas, a South Korean Android Developer.
I tried using KMPAuth and find it very useful! Thanks for your work.
As far as I know, I could find there exists only one option for GoogleAuthCredentials:
serverId,which is used for building
GetGoogleIdOptionwhile other options are fixed in your android implementation like below.KMPAuth/kmpauth-google/src/androidMain/kotlin/com/mmk/kmpauth/google/GoogleAuthUiProviderImpl.kt
Lines 79 to 84 in 2d8c62b
However, there exists an issue for crash in
Credentialswhen you set other options like above: https://stackoverflow.com/questions/78538579/new-google-credential-manager-is-throwing-a-transactiontoolargeexceptionI also got similar issue related to
TransactionTooLargeException, because I already had SAMSUNG Credentials.So I could solve the problem by set
.setFilterByAuthorizedAccounts(true)first for filtering out SAMSUNG Credentials.Also, when you see official recommendation at https://developer.android.com/identity/sign-in/credential-manager-siwg#enable-sign-up,
they guide to check whether there exist previously used accounts by set
.setFilterByAuthorizedAccounts(true)first.If there is no available accounts, it will throw
NoCredentialException.So I suggest adding flow for checking previously used accounts.
This is my first time to participate open source, so please let me know if there exist more steps I should know.
Sincerly, Lukas.